Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement an Unknown variant in the artifact locations #2320

Merged
merged 6 commits into from
Feb 21, 2025

Conversation

sfauvel
Copy link
Collaborator

@sfauvel sfauvel commented Feb 18, 2025

Content

To avoid creating a breaking change in the client whenever a new location is introduced (e.g. decentralized storage).
we add a fallback variant in the artifact location types for the Incremental Cardano DB.

This PR implements an Unknown variant with the #[serde(other)] macro (as explained here) in the artifact locations messages:

  • Immutable locations
  • Ancillary locations
  • Digest locations

The `mithril-client-cli is adapted to not show unknown location.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)

Comments

Issue(s)

Relates to #2293

Copy link

github-actions bot commented Feb 18, 2025

Test Results

    3 files  ± 0     52 suites  ±0   10m 38s ⏱️ +16s
1 657 tests +12  1 657 ✅ +12  0 💤 ±0  0 ❌ ±0 
1 963 runs  +34  1 963 ✅ +34  0 💤 ±0  0 ❌ ±0 

Results for commit 8969387. ± Comparison against base commit be28830.

♻️ This comment has been updated with latest results.

@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch 2 times, most recently from 4fed3dd to 21f5408 Compare February 18, 2025 15:28
@sfauvel sfauvel marked this pull request as ready for review February 18, 2025 15:35
@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch from 21f5408 to fb0d566 Compare February 18, 2025 15:36
@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch from 4bdacfb to 320e040 Compare February 19, 2025 08:47
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch from 320e040 to a6a70c7 Compare February 19, 2025 14:18
@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch 2 times, most recently from 08ec2d4 to 12fbdcd Compare February 20, 2025 08:43
@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch 2 times, most recently from 8057f74 to c71501f Compare February 20, 2025 13:47
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch from c71501f to 569ef01 Compare February 20, 2025 16:29
@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch 2 times, most recently from 4e1913d to ea3cf6c Compare February 20, 2025 16:48
@sfauvel sfauvel force-pushed the sfa/2293/support_evolving_cloud_artifact_locations_type branch from c94651e to 8969387 Compare February 21, 2025 11:10
@sfauvel sfauvel merged commit 01f83cd into main Feb 21, 2025
35 of 39 checks passed
@sfauvel sfauvel deleted the sfa/2293/support_evolving_cloud_artifact_locations_type branch February 21, 2025 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants